-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
module: move unnecessary work for early return #3579
Conversation
The first line of the commit message should start with |
@Trott done! |
@Trott Why should it start with |
@mscdex I always thought (wrongly, it seems!) that we standardize on the nice short directory names in the top-level directory as much as possible. So @zertosh Sorry for the bad suggestion. |
no worries @Trott |
Afaik -- prefixes usually stick to the filenames in |
@nodejs/ctc |
Mostly what @jbergstroem. The name should be the most closely related subsystem you pass to either |
If it's not obvious, just pick one, |
Seems like everyone is Ok on this- even the commit prefix. Merge? EDIT: Meant this for #3578 |
LGTM. |
LGTM |
@jasnell @trevnorris should this be merged? |
LGTM. We could probably change the |
var cacheKey = JSON.stringify({request: request, paths: paths}); | ||
if (Module._pathCache[cacheKey]) { | ||
return Module._pathCache[cacheKey]; | ||
} | ||
|
||
var exts = Object.keys(Module._extensions); | ||
var trailingSlash = (request.slice(-1) === '/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While you're at it, the parenthesis on this line are unnecessary.
`exts` and `trailingSlash` are only used if the path isn't cached.
@cjihrig: I've rebased on top of master and switched the |
Thanks. Last CI was all green, but running one more time just to make sure switching to |
Ci is green green green. |
LGTM |
The exts and trailingSlash variables are only used if the path isn't cached. This commit moves them further down in the code, and changes from var to const. PR-URL: #3579 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Landed in 1285671. Thanks! |
The exts and trailingSlash variables are only used if the path isn't cached. This commit moves them further down in the code, and changes from var to const. PR-URL: #3579 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
The exts and trailingSlash variables are only used if the path isn't cached. This commit moves them further down in the code, and changes from var to const. PR-URL: #3579 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Notable Changes: * Minor performance improvements: - module: move unnecessary work for early return (Andres Suarez) #3579 * Various bug fixes * Various doc fixes * Various test improvements PR-URL: #4626 Reviewed-By: Jeremiah Senkpiel <[email protected]>
The exts and trailingSlash variables are only used if the path isn't cached. This commit moves them further down in the code, and changes from var to const. PR-URL: #3579 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
The exts and trailingSlash variables are only used if the path isn't cached. This commit moves them further down in the code, and changes from var to const. PR-URL: #3579 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
The exts and trailingSlash variables are only used if the path isn't cached. This commit moves them further down in the code, and changes from var to const. PR-URL: nodejs#3579 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
The exts and trailingSlash variables are only used if the path isn't cached. This commit moves them further down in the code, and changes from var to const. PR-URL: nodejs#3579 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
The exts and trailingSlash variables are only used if the path isn't cached. This commit moves them further down in the code, and changes from var to const. PR-URL: nodejs#3579 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
Notable Changes: * Minor performance improvements: - module: move unnecessary work for early return (Andres Suarez) nodejs#3579 * Various bug fixes * Various doc fixes * Various test improvements PR-URL: nodejs#4626 Reviewed-By: Jeremiah Senkpiel <[email protected]>
exts
andtrailingSlash
are only used if the path isn't cached.